-
Notifications
You must be signed in to change notification settings - Fork 114
Create a new fdb-test-utils sub-project for some test utilities #3631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This moves things that were in `fdb-extensions` and needed for testing out into their own test-specific sub-project. The new project doesn't depend on any of the other sub-projects. I left `MultipleTransactions` behind in the testFixtures. It currently depends on `FDBError` (which is part of fdb-extensions), and currently no other project is depending on it, so I felt that leaving it made sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TeamScale is complaining that we should not use varargs
for cartesianProduct
.
I think this is a valid usage of varargs
, as per the explanation of the issue:
Varargs methods are a convenient way to define methods that require a variable number of arguments, but they should not be overused. They can produce confusing results if used inappropriately.
I think we should tolerate this usage and leave it as-is, but wanted am commenting here as a place for discussion.
I will note that cartesianProduct
does have a private overload that takes and returns an incompatible type from the public method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TeamScale is complaining that we should not use varargs
for randomSeeds
.
I think this is a valid usage of varargs
, as per the explanation of the issue:
Varargs methods are a convenient way to define methods that require a variable number of arguments, but they should not be overused. They can produce confusing results if used inappropriately.
I think we should tolerate this usage and leave it as-is, but wanted am commenting here as a place for discussion.
I could not get javadoc to not complain, so I just escaped the @ and used <pre> by itself.
8c29f82
to
0298404
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Maybe we need to actually publish the package befor ethis will work
build.gradle
Outdated
// This is for dependent sub-projects so that their links to things in the Record Layer work | ||
linksOffline "https://foundationdb.github.io/fdb-record-layer/api/fdb-extensions/", "${packageListDir}/fdb-extensions/" | ||
linksOffline "https://foundationdb.github.io/fdb-record-layer/api/fdb-record-layer-core/", "${packageListDir}/fdb-record-layer-core/" | ||
linksOffline "https://foundationdb.github.io/fdb-record-layer/api/fdb-test-utils/", "${packageListDir}/fdb-test-utils/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alecgrieser I followed fdb-extensions
here, and in genpackagelists
below, but that caused the javadoc generation to fail, so I removed them.
My suspicion is that we need to publish the package at least once before this works....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the two teamscale warnings are about var args. I'm personally fine with those as they are used here, if you want to tolerate them
This moves things that were in
fdb-extensions
and needed for testing out into their own test-specific sub-project. The new project doesn't depend on any of the other sub-projects.I left
MultipleTransactions
behind in the testFixtures. It currently depends onFDBError
(which is part of fdb-extensions), and currently no other project is depending on it, so I felt that leaving it made sense.This is in support of: #3574 -- see #3575 (comment)